Dynamic SSL/TLS context switching for Mtls context aggregation between peer bmc#1401
Dynamic SSL/TLS context switching for Mtls context aggregation between peer bmc#1401priyair wants to merge 1 commit intoibm-openbmc:1210from
Conversation
fa93b93 to
70d6721
Compare
70d6721 to
0f88f4d
Compare
0f88f4d to
5468096
Compare
|
|
||
| // Keep context pair alive | ||
| static std::unique_ptr<SslContextPair> s_ctxPair; | ||
| s_ctxPair = std::move(ctxPair); |
There was a problem hiding this comment.
what is the use of static ? we are always recreating ctxPair here
There was a problem hiding this comment.
what is the use of static ? we are always recreating ctxPair here
Removed static and created SslContextPair with both HTTPS and mTLS contexts for TLS handshake.
| } | ||
|
|
||
| // Common helper | ||
| void setVerifyMode(const std::shared_ptr<boost::asio::ssl::context>& ctx, |
There was a problem hiding this comment.
why we need this function?
There was a problem hiding this comment.
setVerifyMode() is called during context initialization (before connections)
|
|
||
| SSL_set_SSL_CTX(ssl, newCtx); | ||
|
|
||
| if (strict) |
There was a problem hiding this comment.
why we not using setVarifymode function declared above?
There was a problem hiding this comment.
why we not using setVarifymode function declared above?
switchSslContext() is called during active SSL handshake (per-connection)
| // Determine if this should be mTLS based on SNI | ||
| if (SSL_client_hello_get0_ext(ssl, TLSEXT_TYPE_server_name, &sni, | ||
| &sniLen) == 1 && | ||
| sniLen > 5) |
There was a problem hiding this comment.
can we move below line in to separate function?
There was a problem hiding this comment.
can we move below line in to separate function?
Done, moved to function std::string hostname =
mtls_SSL_get_servername(ssl, TLSEXT_NAMETYPE_host_name);
| g_contextPairIndex = | ||
| SSL_CTX_get_ex_new_index(0, nullptr, nullptr, nullptr, nullptr); | ||
| } | ||
| } |
There was a problem hiding this comment.
I think 'call-once' may be done more smoothly & clearly by doing like this.
#include <mutex>
std::once_flag exDataIndex_flag;
static void initializeExDataIndex()
{
std::call_once( exDataIndex_flag,
[](){
g_contextPairIndex =
SSL_CTX_get_ex_new_index(0, nullptr, nullptr, nullptr, nullptr);
});
}
| // Create mTLS server context | ||
| static std::shared_ptr<boost::asio::ssl::context> getSslMtlsServerContext() | ||
| { | ||
| using namespace boost::asio::ssl; |
There was a problem hiding this comment.
Would it be better to use the full namespace for the related ones for the clear references?
- boost::asio::ssl::context::tls_server
There was a problem hiding this comment.
Would it be better to use the full namespace for the related ones for the clear references?
- boost::asio::ssl::context::tls_server
Done
This patch implements dynamic SSL/TLS context switching
in bmcweb. The implementation maintains two separate SSL
contexts:
- HTTPS context: standard TLS with optional client
certificate
- mTLS context: mutual TLS requiring mandatory client
certificate authentication
The server selects the appropriate context based on the
client's Server Name Indication (SNI). This enables
secure peer-to-peer Redfish aggregation while continuing
to support standard HTTPS clients on same port.
Tested By:
'''
- Configured peer BMC aggregation using JSON
configuration files
- Verified requests from client BMCs with
TLS strict mode both enabled and disabled
- Tested with client certificates and
authentication tokens
- HTTPS and mTLS connections are correctly
established and switched based on SNI
1. Token-based authentication with tlsStrict=false:
curl -k -H "X-Auth-Token: $bmc_token24" -X GET \
https://<bmc>/redfish/v1/Systems
2. Certificate upload to Truststore:
curl -k -H "X-Auth-Token: $bmc_token26" -X POST \
https://<bmc26>/redfish/v1/bmc/Truststore/Certificates \
-H "Content-Type: application/json" \
-d '{"CertificateType": "PEM",
"CertificateString": "'"$CERT_STRING"'"}'
3. Certificate replacement:
curl -k -H "X-Auth-Token: $bmc_token26" -X POST \
https://<bmc26>/redfish/v1/bmc/CertificateService.ReplaceCertificate \
-H "Content-Type: application/json" -d '{
"CertificateType": "PEM",
"CertificateString": "'"$CERT_STRING"'",
"CertificateUri": {
"@odata.id": "/redfish/v1/HTTPS/Certificates/1"
}
}'
4. mTLS authentication with tlsStrict=false:
curl -v --cert client.pem \
--cacert ../serverCA/ca.crt \
https://<bmc>/redfish/v1/Systems
5. Token-based authentication with tlsStrict=true:
curl -k -H "X-Auth-Token: $bmc_token24" -X GET \
https://<bmc>/redfish/v1/Systems
6. mTLS authentication with tlsStrict=true:
curl -v --cert client.pem \
--cacert ../serverCA/ca.crt \
https://<bmc>/redfish/v1/Systems
'''
Change-Id: I49c8f3d69d4b91a3a1ebe9b5f001943d5f1f47f8
5468096 to
ca209e6
Compare
baemyung
left a comment
There was a problem hiding this comment.
It looks good to me.
BTW, I think it is better to add upstream gerrit commit link here.
|
gtmills
left a comment
There was a problem hiding this comment.
Can we keep going upstream here?
This pull request supersedes #1395.
This patch implements dynamic SSL/TLS context switching
in bmcweb. The implementation maintains two separate SSL
contexts: